[CHA-2563] Add retention policy integration tests#227
Merged
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughExtended REST client APIs across chat, common, moderation, video, and feeds modules with new retention-policy endpoints, importer-storage management, SIP authentication, comment restoration, and collection queries. Updated method signatures with optional parameters for chat preferences, activity metrics, and SIP configurations. Added supporting dataclass models for retention policies and cleanup runs. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
getstream/models/__init__.py (1)
1825-1828: Note: Default type value applies to only one of four handled event types.
AsyncExportErrorEventis used forexport.bulk_image_moderation.error,export.channels.error,export.moderation_logs.error, andexport.users.error(pergetstream/webhook.pymapping). The default now specifically matchesbulk_image_moderation.error.This is fine for deserialization (the type comes from the payload), but if code instantiates this class directly without specifying
type, it will default to bulk image moderation regardless of the actual error context.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@getstream/models/__init__.py` around lines 1825 - 1828, The AsyncExportErrorEvent class currently sets type: str = dc_field(default="export.bulk_image_moderation.error", metadata=dc_config(field_name="type")), which biases direct instantiation to the bulk_image_moderation event; remove the hardcoded default so the field is required (e.g., change to type: str = dc_field(metadata=dc_config(field_name="type")) or use an explicit empty/default-less dc_field) and add optional runtime validation if needed; reference AsyncExportErrorEvent, the type field declaration, dc_field/dc_config usage, and the getstream/webhook.py mapping to ensure callers supply the correct specific event type when constructing instances.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@getstream/models/__init__.py`:
- Around line 1825-1828: The AsyncExportErrorEvent class currently sets type:
str = dc_field(default="export.bulk_image_moderation.error",
metadata=dc_config(field_name="type")), which biases direct instantiation to the
bulk_image_moderation event; remove the hardcoded default so the field is
required (e.g., change to type: str =
dc_field(metadata=dc_config(field_name="type")) or use an explicit
empty/default-less dc_field) and add optional runtime validation if needed;
reference AsyncExportErrorEvent, the type field declaration, dc_field/dc_config
usage, and the getstream/webhook.py mapping to ensure callers supply the correct
specific event type when constructing instances.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 34692f6d-7d29-403d-9747-43880155166d
📒 Files selected for processing (13)
getstream/chat/async_rest_client.pygetstream/chat/rest_client.pygetstream/common/async_rest_client.pygetstream/common/rest_client.pygetstream/feeds/rest_client.pygetstream/models/__init__.pygetstream/moderation/async_rest_client.pygetstream/moderation/rest_client.pygetstream/tests/test_webhook.pygetstream/video/async_rest_client.pygetstream/video/rest_client.pygetstream/webhook.pytests/test_chat_misc.py
hifaizsk
force-pushed
the
feature/cha-2563-retention-policy-endpoints
branch
from
ceabba6 to
004840e
Compare
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Regenerated SDK from latest serverside-api.yaml spec. Updated retention policy integration test to only test get_retention_policy_runs endpoint. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@getstream/moderation/async_rest_client.py`:
- Around line 141-146: Both methods reference the
"/api/v2/moderation/moderation_rule/{id}" route but never accept or pass an id,
so the {id} placeholder can never be populated; update delete_moderation_rule
and get_moderation_rule to accept an id parameter (e.g., id: str) and call
self.delete/self.get with path_params={"id": id} (keep the
telemetry.operation_name on get_moderation_rule unchanged) so the route is
invoked with the actual id.
- Around line 62-63: The public async method signatures (notably check and
submit_action) have new optional parameters inserted mid-parameter-list which
breaks callers using positional args; move any newly added optional parameters
(e.g., content_published_at for check and escalate/submit-related flags for
submit_action) to the end of the public function signatures preserving original
parameter order, update the call that builds the request object
(CheckRequest(...) in check and the corresponding request builder in
submit_action) to use keyword arguments or the new parameter order, and ensure
type annotations and default None values remain unchanged so existing positional
callers continue to work.
In `@getstream/moderation/rest_client.py`:
- Around line 140-146: The methods delete_moderation_rule and
get_moderation_rule are missing an id parameter and are not passing path_params
for the "{id}" placeholder; update both methods (and their async counterparts)
to accept id: str (or appropriate type), include it in path_params when calling
self.delete/self.get (e.g., path_params={"id": id}), and ensure the method
signatures and return types remain the same (refer to delete_moderation_rule,
get_moderation_rule and their async versions to mirror the pattern used by
get_appeal).
In `@getstream/video/rest_client.py`:
- Around line 487-489: The signature of resolve_sip_inbound changed breaking
positional compatibility by inserting trunk_id before challenge; restore
backward-compatible parameter order by moving trunk_id to the end of the
optional parameters list in resolve_sip_inbound (and the mirrored method in
async_rest_client), so the signature becomes (sip_caller_number,
sip_trunk_number, routing_number=None, challenge=None, sip_headers=None,
trunk_id=None); update the OpenAPI codegen template that produces these methods
so future generated code keeps trunk_id last and ensure the body construction
still uses the trunk_id variable name when present.
In `@tests/test_chat_misc.py`:
- Around line 605-614: Replace the lone test_get_retention_policy_runs with a
test class that exercises the full retention-policy CRUD flow (call
client.chat.set_retention_policy, then client.chat.get_retention_policy,
client.chat.get_retention_policy_runs, and finally
client.chat.delete_retention_policy) so wiring issues in rest_client.py surface;
group these tests in a class (e.g., TestRetentionPolicyCRUD) and assert expected
responses at each step; only skip the entire class when the API returns an
explicit feature-disabled response (detect a specific error code/message from
client.chat.* calls rather than any generic "404" / "not found"), and ensure
cleanup (delete) runs when setup succeeded.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2bd5112e-6c63-4bf7-83e3-d01dcfbbadb2
📒 Files selected for processing (18)
getstream/chat/async_channel.pygetstream/chat/async_rest_client.pygetstream/chat/channel.pygetstream/chat/rest_client.pygetstream/common/async_rest_client.pygetstream/common/rest_client.pygetstream/feeds/feeds.pygetstream/feeds/rest_client.pygetstream/models/__init__.pygetstream/moderation/async_rest_client.pygetstream/moderation/rest_client.pygetstream/tests/test_webhook.pygetstream/video/async_call.pygetstream/video/async_rest_client.pygetstream/video/call.pygetstream/video/rest_client.pygetstream/webhook.pytests/test_chat_misc.py
✅ Files skipped from review due to trivial changes (3)
- getstream/feeds/feeds.py
- getstream/chat/async_channel.py
- getstream/chat/channel.py
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
getstream/feeds/rest_client.py (1)
1963-1977:⚠️ Potential issue | 🟡 MinorSame breaking change as
unfollow_batch: parameter type changed toUnfollowPair.This has the same implications as the
unfollow_batchchange above. Existing callers usingFollowPairwill need to migrate toUnfollowPair.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@getstream/feeds/rest_client.py` around lines 1963 - 1977, get_or_create_unfollows currently requires List[UnfollowPair], which breaks callers still passing List[FollowPair]; update the method signature and body to accept both types (e.g., follows: List[Union[FollowPair, UnfollowPair]] or keep typing but detect FollowPair at runtime) and convert any FollowPair instances to UnfollowPair before building UnfollowBatchRequest; specifically adjust get_or_create_unfollows to handle FollowPair -> UnfollowPair conversion (or normalization) so existing callers continue to work while new callers can pass UnfollowPair.
♻️ Duplicate comments (2)
getstream/moderation/rest_client.py (1)
152-153:⚠️ Potential issue | 🟠 MajorPreserve positional-call compatibility by appending new optional params at the end.
Line 152 and Line 560 add optional args mid-signature, which can misroute positional arguments in existing integrations.
Proposed signature ordering fix
def check( self, entity_creator_id: str, entity_id: str, entity_type: str, config_key: Optional[str] = None, config_team: Optional[str] = None, - content_published_at: Optional[datetime] = None, test_mode: Optional[bool] = None, user_id: Optional[str] = None, config: Optional[ModerationConfig] = None, moderation_payload: Optional[ModerationPayload] = None, options: Optional[Dict[str, object]] = None, user: Optional[UserRequest] = None, + content_published_at: Optional[datetime] = None, ) -> StreamResponse[CheckResponse]:def submit_action( self, action_type: str, appeal_id: Optional[str] = None, item_id: Optional[str] = None, user_id: Optional[str] = None, ban: Optional[BanActionRequestPayload] = None, block: Optional[BlockActionRequestPayload] = None, custom: Optional[CustomActionRequestPayload] = None, delete_activity: Optional[DeleteActivityRequestPayload] = None, delete_comment: Optional[DeleteCommentRequestPayload] = None, delete_message: Optional[DeleteMessageRequestPayload] = None, delete_reaction: Optional[DeleteReactionRequestPayload] = None, delete_user: Optional[DeleteUserRequestPayload] = None, - escalate: Optional[EscalatePayload] = None, flag: Optional[FlagRequest] = None, mark_reviewed: Optional[MarkReviewedRequestPayload] = None, reject_appeal: Optional[RejectAppealRequestPayload] = None, restore: Optional[RestoreActionRequestPayload] = None, shadow_block: Optional[ShadowBlockActionRequestPayload] = None, unban: Optional[UnbanActionRequestPayload] = None, unblock: Optional[UnblockActionRequestPayload] = None, user: Optional[UserRequest] = None, + escalate: Optional[EscalatePayload] = None, ) -> StreamResponse[SubmitActionResponse]:Also applies to: 166-167, 560-561, 583-584
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@getstream/moderation/rest_client.py` around lines 152 - 153, The new optional parameters content_published_at and test_mode were inserted mid-signature and can break positional-call compatibility; move these optional params to the end of the function/method parameter lists wherever they appear (the signatures shown with content_published_at/test_mode at lines around 152-153, 166-167, 560-561, and 583-584), update the function/type hints and docstrings accordingly, and adjust any internal callers/tests that relied on the previous positional ordering so callers continue to work with positional args.getstream/moderation/async_rest_client.py (1)
154-155:⚠️ Potential issue | 🟠 MajorKeep new optional params at the end of public signatures to avoid positional-call breakage.
Line 154 and Line 568 introduce new optional args in the middle of existing parameter lists. Positional callers will silently bind values to the wrong parameters.
Proposed compatibility-safe signature order
async def check( self, entity_creator_id: str, entity_id: str, entity_type: str, config_key: Optional[str] = None, config_team: Optional[str] = None, - content_published_at: Optional[datetime] = None, test_mode: Optional[bool] = None, user_id: Optional[str] = None, config: Optional[ModerationConfig] = None, moderation_payload: Optional[ModerationPayload] = None, options: Optional[Dict[str, object]] = None, user: Optional[UserRequest] = None, + content_published_at: Optional[datetime] = None, ) -> StreamResponse[CheckResponse]:async def submit_action( self, action_type: str, appeal_id: Optional[str] = None, item_id: Optional[str] = None, user_id: Optional[str] = None, ban: Optional[BanActionRequestPayload] = None, block: Optional[BlockActionRequestPayload] = None, custom: Optional[CustomActionRequestPayload] = None, delete_activity: Optional[DeleteActivityRequestPayload] = None, delete_comment: Optional[DeleteCommentRequestPayload] = None, delete_message: Optional[DeleteMessageRequestPayload] = None, delete_reaction: Optional[DeleteReactionRequestPayload] = None, delete_user: Optional[DeleteUserRequestPayload] = None, - escalate: Optional[EscalatePayload] = None, flag: Optional[FlagRequest] = None, mark_reviewed: Optional[MarkReviewedRequestPayload] = None, reject_appeal: Optional[RejectAppealRequestPayload] = None, restore: Optional[RestoreActionRequestPayload] = None, shadow_block: Optional[ShadowBlockActionRequestPayload] = None, unban: Optional[UnbanActionRequestPayload] = None, unblock: Optional[UnblockActionRequestPayload] = None, user: Optional[UserRequest] = None, + escalate: Optional[EscalatePayload] = None, ) -> StreamResponse[SubmitActionResponse]:Also applies to: 168-169, 568-569, 591-592
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@getstream/moderation/async_rest_client.py` around lines 154 - 155, New optional parameters content_published_at and test_mode were inserted in the middle of public function signatures in getstream/moderation/async_rest_client.py which will break positional callers; locate every public function or method in that file that currently has content_published_at and/or test_mode in its parameter list (the occurrences noted around the earlier diff blocks) and move those optional parameters to the end of the parameter list (preserving Optional[datetime] and Optional[bool] types and default None values), update any corresponding type hints and docstrings, and ensure any internal call sites use keyword arguments or are updated to the new ordering so existing positional calls keep the original binding behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@getstream/feeds/rest_client.py`:
- Around line 1963-1977: get_or_create_unfollows currently requires
List[UnfollowPair], which breaks callers still passing List[FollowPair]; update
the method signature and body to accept both types (e.g., follows:
List[Union[FollowPair, UnfollowPair]] or keep typing but detect FollowPair at
runtime) and convert any FollowPair instances to UnfollowPair before building
UnfollowBatchRequest; specifically adjust get_or_create_unfollows to handle
FollowPair -> UnfollowPair conversion (or normalization) so existing callers
continue to work while new callers can pass UnfollowPair.
---
Duplicate comments:
In `@getstream/moderation/async_rest_client.py`:
- Around line 154-155: New optional parameters content_published_at and
test_mode were inserted in the middle of public function signatures in
getstream/moderation/async_rest_client.py which will break positional callers;
locate every public function or method in that file that currently has
content_published_at and/or test_mode in its parameter list (the occurrences
noted around the earlier diff blocks) and move those optional parameters to the
end of the parameter list (preserving Optional[datetime] and Optional[bool]
types and default None values), update any corresponding type hints and
docstrings, and ensure any internal call sites use keyword arguments or are
updated to the new ordering so existing positional calls keep the original
binding behavior.
In `@getstream/moderation/rest_client.py`:
- Around line 152-153: The new optional parameters content_published_at and
test_mode were inserted mid-signature and can break positional-call
compatibility; move these optional params to the end of the function/method
parameter lists wherever they appear (the signatures shown with
content_published_at/test_mode at lines around 152-153, 166-167, 560-561, and
583-584), update the function/type hints and docstrings accordingly, and adjust
any internal callers/tests that relied on the previous positional ordering so
callers continue to work with positional args.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 365f11b6-7dae-4afd-9dd3-02471e4d5259
📒 Files selected for processing (12)
getstream/chat/async_rest_client.pygetstream/chat/rest_client.pygetstream/common/async_rest_client.pygetstream/common/rest_client.pygetstream/feeds/rest_client.pygetstream/models/__init__.pygetstream/moderation/async_rest_client.pygetstream/moderation/rest_client.pygetstream/tests/test_webhook.pygetstream/video/async_rest_client.pygetstream/video/rest_client.pygetstream/webhook.py
🚧 Files skipped from review as they are similar to previous changes (3)
- getstream/webhook.py
- getstream/chat/rest_client.py
- getstream/video/async_rest_client.py
mogita
approved these changes
Mar 31, 2026
Remove duplicate AsyncExportErrorEvent imports in webhook.py, remove unused json import in test_webhook.py, and apply ruff formatting. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Ticket
Summary
SetRetentionPolicy,GetRetentionPolicy,GetRetentionPolicyRuns,DeleteRetentionPolicyChecklist
🤖 Generated with Claude Code
Summary by CodeRabbit